Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust nginx log level according to modsecurity severity level #243

Closed
wants to merge 1 commit into from

Conversation

qix67
Copy link

@qix67 qix67 commented Mar 22, 2021

Without this patch, whatever severity level in modsecurity message, it
appears as 'error' in nginx error.log file.

This patch modifies this behavior. When a severity level is present, it
is used as nginx log level else 'error' nginx log level is used.

Without this patch, whatever severity level in modsecurity message, it
appears as 'error' in nginx error.log file.

This patch modifies this behavior. When a severity level is present, it
is used as nginx log level else 'error' nginx log level is used.
@martinhsv
Copy link
Contributor

Hello @qix67 ,

There are a few open issues with a somewhat similar goal (by which I mean providing additional control over the log level of lines written to error.log).

In general I'm at least somewhat hesitant to equate ModSecurity's rule 'severity' with the log level in a web server's error.log file. (See owasp-modsecurity/ModSecurity#2748 (comment) for some reasoning.)

Moreover, if we did decide to do something along those lines, I think it would be preferable to implement it through a specific variable, rather than parsing log lines. Parsing log line content has a chance of not producing the intended results. I haven't tried the proposed code here, but, for example, what if the 11-character string '[severity "' were to appear somewhere else in the rule output? Perhaps the admin, for obscure reasons included text like that in the rule's "msg" action?

@martinhsv martinhsv closed this Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants